Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swift db drivers #526

Closed
wants to merge 12 commits into from
Closed

Conversation

deepfryed
Copy link

Update the Swift adapters for Sequel to use

https://github.com/deepfryed/swift-db-mysql
https://github.com/deepfryed/swift-db-postgres
https://github.com/deepfryed/swift-db-sqlite3

All three have been released to Rubygems and the corresponding specs all pass.

I have to look into all the pending tests for swift and try to get most of them to pass.

Thanks

@jeremyevans
Copy link
Owner

Briefly reviewing this now. Some notes:

We generally don't do $stderr.puts for explanatory error messages in exceptions. It should be obvious to users that if you get a LoadError when requiring swift/db/mysql, you need to install a library, and the documentation you added to opening_databases.rdoc should cover that.

You are removing ::SwiftError from Postgres::CONVERTED_EXCEPTIONS instead of replacing it with ::Swift::Error. What's the reasoning behind that?

Other than those things, this looks good, and I should be able to merge and test it tomorrow.

Thanks for the help!

@jeremyevans
Copy link
Owner

Also, looking at the new drivers, it doesn't look like dbic++ is used anymore? Is that correct? If so, what's the status of dbic++ (I'm the OpenBSD maintainer of the dbic++ port)?

@deepfryed
Copy link
Author

We generally don't do $stderr.puts for explanatory error messages in exceptions.

removed the LoadError puts

"You are removing ::SwiftError from Postgres::CONVERTED_EXCEPTIONS instead of replacing it with ::Swift::Error. What's the reasoning behind that?"

I must have done it because Swift::Error required loading the swift/db/postgres first. I've just rearranged the require order and put that code back in sequel/adapters/swift/postgres

it doesn't look like dbic++ is used anymore?

Yes, it doesn't. Decoupling swift from the binary adapters was in the pipeline for a while and this rewrite lead to more cleaner and easier to maintain codebase.

what's the status of dbic++

I'm still maintaining dbic++ as it is still a library I use for writing c++ code.

@jeremyevans
Copy link
Owner

Thanks for the changes and the info. I'll try to get this merged and tested tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants